-
Notifications
You must be signed in to change notification settings - Fork 851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove passive task resend logic #6906
base: main
Are you sure you want to change the base?
Conversation
registry namespace.Registry, | ||
) bool { | ||
workflowKey := taskWorkflowKey(taskInfo) | ||
remoteClusterName, err := getRemoteClusterName( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: sourceCluster to match the method name?
if err != nil { | ||
return true | ||
} | ||
_, err = remoteAdminClient.DescribeMutableState(ctx, &adminservice.DescribeMutableStateRequest{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use DescribeWorkflowExecution?
@@ -166,7 +160,7 @@ func (t *timerQueueStandbyTaskExecutor) executeUserTimerTimeoutTask( | |||
timerTask.GetVisibilityTime(), | |||
timerSequenceID.Timestamp, | |||
) { | |||
return getHistoryResendInfo(mutableState) | |||
return &struct{}{}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: seem like we don't need extra data in the struct. could we just use a boolean?
What changed?
Remove passive task resend logic
Why?
Resend is best effort
How did you test it?
unit test
Potential risks
passive task may disappear
Documentation
no document
Is hotfix candidate?
no